feat: warn on unsaved changes#259
Conversation
when leaving case-detail (#217)
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 44 minutes and 33 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request introduces unsaved changes detection and navigation blocking for the case detail view. The App component uses a ref to track dirty state and blocks in-app navigation when changes exist, while CaseDetail reports edit status and auto-saves on unmount. Changes
Sequence DiagramsequenceDiagram
actor User
participant App
participant CaseDetail
participant MedicalRecordService
User->>CaseDetail: Edit title/description or type message
CaseDetail->>App: onDirtyChange(true)
App->>App: Update caseDetailDirtyRef.current
User->>App: Attempt to navigate away from case-detail
App->>App: Check caseDetailDirtyRef.current
alt Dirty state is true
App->>User: Show confirmation dialog
alt User confirms
User->>App: Click OK
App->>App: Clear caseDetailDirtyRef.current
App->>CaseDetail: Trigger unmount
else User cancels
User->>App: Click Cancel
Note over App: Navigation blocked
end
else Dirty state is false
App->>CaseDetail: Proceed with unmount
end
CaseDetail->>App: onDirtyChange(false)
CaseDetail->>MedicalRecordService: Auto-save edited title & description
MedicalRecordService-->>CaseDetail: Update complete (or error ignored)
CaseDetail->>User: Component unmounted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/App.jsx (1)
112-123:⚠️ Potential issue | 🟠 MajorIn-page "Tillbaka" button bypasses the unsaved-changes prompt.
onNavigate(lines 276–282) guards Layout-level navigation withwindow.confirm, butgoBackToDashboard— invoked byCaseDetail's back button and byonGoToPet— unconditionally clearscaseDetailDirtyRef.currentand changes the view. Users leaving via the back arrow or the patient-card link will never see the warning, and (combined with the unmount auto-save inCaseDetail.jsx) their edits will be silently persisted.Consider centralizing the guard:
♻️ Suggested change
const goBackToDashboard = () => { + if (currentView === 'case-detail' && caseDetailDirtyRef.current) { + if (!window.confirm('Du har osparade ändringar. Vill du lämna sidan?')) return; + } caseDetailDirtyRef.current = false; setSelectedPet(null); ... };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/App.jsx` around lines 112 - 123, The goBackToDashboard handler unconditionally clears caseDetailDirtyRef.current and switches views, bypassing the unsaved-changes guard implemented in onNavigate; update goBackToDashboard (and any callers such as onGoToPet/CaseDetail back actions) to reuse the same confirmation logic as onNavigate by checking caseDetailDirtyRef.current and calling window.confirm before clearing the ref and calling setCurrentView/setSelectedPet/setSelectedRecord, or refactor the onNavigate guard into a shared helper (e.g., confirmAndNavigate) and call that from goBackToDashboard so all in-page navigations prompt consistently.
🧹 Nitpick comments (1)
frontend/src/App.jsx (1)
276-282: Nit:onNavigatere-creates on every render.The inline arrow is fine functionally, but since
Layoutlikely passes it to nav items, it forces them to re-render each time. Wrapping inuseCallback(with dependency oncurrentView) would be a cheap improvement. Optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/App.jsx` around lines 276 - 282, The inline onNavigate arrow prop re-creates every render causing child re-renders; extract it into a stable callback using React.useCallback (e.g. create a handleNavigate that contains the current logic referencing currentView, caseDetailDirtyRef.current and setCurrentView) and pass that handleNavigate to Layout/onNavigate with dependencies set to [currentView] (refs need not be in deps) so the function identity is stable across renders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/pages/CaseDetail.jsx`:
- Around line 54-65: The cleanup effect currently auto-saves silently on unmount
(useEffect cleanup) by calling medicalRecordService.update with
editedTitleRef/editedDescriptionRef and swallowing errors, which causes
unexpected persistence and lost error visibility; change the cleanup so it does
NOT auto-save by default — only clear the dirty flag via onDirtyChange(false)
and avoid calling medicalRecordService.update unless an explicit
opt-in/draft-save flag is present (or the user clicked an explicit “save”
action), and if you must call update then surface failures (show
toast/console.error) instead of .catch(()=>{}); also add caseData to the effect
deps (or guard against missing caseData in cleanup) so caseData.id is not
dereferenced if falsy at mount.
- Around line 36-39: The current dirty-state logic marks the form dirty whenever
isEditing is true even if editedTitle/editedDescription match caseData, so
update the useEffect that calls onDirtyChange (and the similar beforeunload
handler and unmount auto-save guard) to compute dirty as:
newMessage.trim().length > 0 OR (isEditing AND (editedTitle !== caseData.title
OR editedDescription !== caseData.description)); replace the simple isEditing
check with this comparison so entering edit mode without changes does not set
the dirty flag and apply the same comparison logic to the beforeunload and
on-unmount autosave guards.
---
Outside diff comments:
In `@frontend/src/App.jsx`:
- Around line 112-123: The goBackToDashboard handler unconditionally clears
caseDetailDirtyRef.current and switches views, bypassing the unsaved-changes
guard implemented in onNavigate; update goBackToDashboard (and any callers such
as onGoToPet/CaseDetail back actions) to reuse the same confirmation logic as
onNavigate by checking caseDetailDirtyRef.current and calling window.confirm
before clearing the ref and calling
setCurrentView/setSelectedPet/setSelectedRecord, or refactor the onNavigate
guard into a shared helper (e.g., confirmAndNavigate) and call that from
goBackToDashboard so all in-page navigations prompt consistently.
---
Nitpick comments:
In `@frontend/src/App.jsx`:
- Around line 276-282: The inline onNavigate arrow prop re-creates every render
causing child re-renders; extract it into a stable callback using
React.useCallback (e.g. create a handleNavigate that contains the current logic
referencing currentView, caseDetailDirtyRef.current and setCurrentView) and pass
that handleNavigate to Layout/onNavigate with dependencies set to [currentView]
(refs need not be in deps) so the function identity is stable across renders.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f2c63ad0-ee58-412e-8ac1-72a0457a0ca6
📒 Files selected for processing (2)
frontend/src/App.jsxfrontend/src/pages/CaseDetail.jsx
1 participant
when leaving case-detail
(#217)
Summary by CodeRabbit